Replace Content-Type with Accept on bodyless GET proxy routes#1243
Replace Content-Type with Accept on bodyless GET proxy routes#1243vishsanghishetty wants to merge 1 commit intoambient-code:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (13)
📝 WalkthroughWalkthroughReplaced incorrect outbound Changes
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4052003 to
ee6ea92
Compare
b9360dc to
57325a7
Compare
buildForwardHeaders now defaults to Accept: application/json instead of Content-Type, since most callers are GET proxies with no body. POST/PUT routes that send a body now set Content-Type explicitly. closes ambient-code#1002 Signed-off-by: Vishali <vsanghis@redhat.com>
57325a7 to
1e49f42
Compare
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
|
@jeremyeder could you take a look when you get a chance? |
Closes #1002
What changed
The root cause is
buildForwardHeadersinlib/auth.ts— it unconditionally setsContent-Type: application/jsonon every outbound request, even GET proxies with no body. Changed it to default toAccept: application/jsoninstead, which fixes all ~40 GET routes that use the helper in one shot.On top of that, replaced the literal
Content-TypewithAccepton the 5 routes called out in the issue (version,cluster-info,settingsGET,workflows/ootb,feature-flags).Since POST/PUT routes that send a body still need
Content-Type, added it explicitly to the 14 routes that were relying on the helper for it (projects,permissions,keys, auth connect routes,agentic-sessions,workflow,repos,configure-remote,feature-flag override,forks). Routes that already had explicitContent-Type(scheduled-sessions,runner-secrets,integration-secrets,agui,mcp/invoke, workspace paths) were unaffected.Scope
buildForwardHeadershelperContent-Type→AcceptContent-TypeAcceptContent-TypeContent-TypealreadyFull audit of all 94 route files under
src/app/api/— nothing missed.How I tested
Static analysis —
tsc --noEmit,eslinton all 20 changed files,vitest run(631 passed, 0 failures).Live testing against the Kind cluster — ran the frontend locally (Next.js dev server on port 3000) with the backend port-forwarded from the
ambient-mainKind cluster, then curled every modified route type through the proxy layer:/api/version/api/cluster-info/api/workflows/ootb/api/projects/api/projectsThe POST test confirms
Content-Type: application/jsonis still being sent on mutation routes — the backend parsed the JSON body and returned a meaningful validation error, not a "can't parse request" error.Summary by CodeRabbit